validate redirect origin in oidc rp sign-in completion#3241
Conversation
Signed-off-by: dxbjavid <dxbjavid@gmail.com>
| MultivaluedMap<String, String> state = oidcContext.getState(); | ||
| String location = state != null ? state.getFirst("state") : null; | ||
| if (location == null && defaultLocation != null) { | ||
| if (location != null) { |
There was a problem hiding this comment.
@coheigea need your advice please, the usage of state for redirect URL looks very suspicious, I haven't found any references in OIDC RP specs of such designation, is it correct or I am missing something? thank you
There was a problem hiding this comment.
you're right that it isn't an OIDC concept, the spec treats state as opaque. this is cxf's own reuse of the parameter name rather than anything from the RP specs.
it comes from OidcRpAuthenticationFilter: when addRequestUriAsRedirectQuery is set, the filter writes the original request uri into a query parameter it happens to call state (redirectBuilder.queryParam("state", rc.getUriInfo().getRequestUri().toString())), and completeAuthentication later reads that same state back to send the browser to where it was originally heading. so in normal use the value is always the application's own request uri, which is why the legitimate case is same-origin.
the problem is that toRequestState copies every current request parameter into the context state, so /rp/complete?state=https://evil.example against an already-authenticated session is read back verbatim and returned as a 303 Location, an open redirect. the origin check just keeps that redirect within the application's own scheme and authority. if you'd rather the validation lived in the filter where the value is first written, i'm happy to move it there.
There was a problem hiding this comment.
. if you'd rather the validation lived in the filter where the value is first written, i'm happy to move it there.
Oh thanks for clarifying @dxbjavid , I think hardening filter would make sense, also making sure the state is not colliding with the request state (may be filtering here will help), thanks again
2 participants
when the rp sign-in flow completes, OidcRpAuthenticationService redirects the browser to the state value, which OidcRpAuthenticationFilter copies straight from the current request parameters, so a request such as /rp/complete?state=https://evil.example against an authenticated session returns a 303 to an arbitrary external host, an open redirect. the legitimate value is always the application's own request uri, so completeAuthentication now only honours a location that is relative or shares the same scheme and authority as the base path, otherwise it falls back to the configured default location. the added test covers the cross-origin, protocol-relative and userinfo-host variants.